fix(internal/librarian): update-image should skip libraries where generate is blocked by config (#2823)

`update-image` command now skips a library if it is configured with
`generate_blocked` in config.yaml.
Also done minor refactoring to reuse logic in identifying blocked
libraries.

Fixes #2773

Author: zhumin8

internal/config/librarian_config.go

@@ -79,6 +79,15 @@ func (g *LibrarianConfig) LibraryConfigFor(LibraryID string) *LibraryConfig {
 	return nil
 }
 
+// IsGenerationBlocked returns true if the library is configured to block generation.
+func (g *LibrarianConfig) IsGenerationBlocked(libraryID string) bool {
+	if g == nil {
+		return false
+	}
+	libConfig := g.LibraryConfigFor(libraryID)
+	return libConfig != nil && libConfig.GenerateBlocked
+}
+
 // GetGlobalFiles returns the global files defined in the librarian config.
 func (g *LibrarianConfig) GetGlobalFiles() []string {
 	var globalFiles []string

internal/config/librarian_config_test.go

@@ -218,3 +218,56 @@ func TestGetGlobalFiles(t *testing.T) {
 		})
 	}
 }
+
+func TestIsGenerationBlocked(t *testing.T) {
+	for _, test := range []struct {
+		name      string
+		config    *LibrarianConfig
+		libraryID string
+		want      bool
+	}{
+		{
+			name:      "nil config",
+			config:    nil,
+			libraryID: "lib1",
+			want:      false,
+		},
+		{
+			name: "library not in config",
+			config: &LibrarianConfig{
+				Libraries: []*LibraryConfig{
+					{LibraryID: "lib2", GenerateBlocked: true},
+				},
+			},
+			libraryID: "lib1",
+			want:      false,
+		},
+		{
+			name: "library in config, generate_blocked is false",
+			config: &LibrarianConfig{
+				Libraries: []*LibraryConfig{
+					{LibraryID: "lib1", GenerateBlocked: false},
+				},
+			},
+			libraryID: "lib1",
+			want:      false,
+		},
+		{
+			name: "library in config, generate_blocked is true",
+			config: &LibrarianConfig{
+				Libraries: []*LibraryConfig{
+					{LibraryID: "lib1", GenerateBlocked: true},
+				},
+			},
+			libraryID: "lib1",
+			want:      true,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			got := test.config.IsGenerationBlocked(test.libraryID)
+			if got != test.want {
+				t.Errorf("IsGenerationBlocked() = %v, want %v", got, test.want)
+			}
+		})
+	}
+}

internal/librarian/generate_command.go

@@ -426,12 +426,9 @@ func setAllAPIStatus(state *config.LibrarianState, status string) {
 func (r *generateRunner) shouldGenerate(library *config.LibraryState) (bool, error) {
 	// If the library has a manual configuration which indicates generation is blocked,
 	// the library is skipped.
-	if r.librarianConfig != nil {
-		libConfig := r.librarianConfig.LibraryConfigFor(library.ID)
-		if libConfig != nil && libConfig.GenerateBlocked {
-			slog.Info("library has generate_blocked, skipping", "id", library.ID)
-			return false, nil
-		}
+	if r.librarianConfig.IsGenerationBlocked(library.ID) {
+		slog.Info("library has generate_blocked, skipping", "id", library.ID)
+		return false, nil
 	}
 
 	// If the library has no APIs, it is skipped.

internal/librarian/test_container_generate.go

@@ -163,12 +163,10 @@ func (r *testGenerateRunner) testSingleLibrary(ctx context.Context, libraryID, s
 		return fmt.Errorf("library %q not found in state", libraryID)
 	}
 
-	if r.librarianConfig != nil {
-		libConfig := r.librarianConfig.LibraryConfigFor(libraryID)
-		if libConfig != nil && libConfig.GenerateBlocked {
-			return errGenerateBlocked
-		}
+	if r.librarianConfig.IsGenerationBlocked(libraryID) {
+		return errGenerateBlocked
 	}
+
 	protoFileToGUIDs, err := r.prepareForGenerateTest(libraryState, libraryID)
 	if err != nil {
 		return fmt.Errorf("failed in test preparing steps: %w", err)

internal/librarian/update_image.go

@@ -118,13 +118,19 @@ func (r *updateImageRunner) run(ctx context.Context) error {
 	// For each library, run generation at the previous commit
 	var failedGenerations []*config.LibraryState
 	var successfulGenerations []*config.LibraryState
+	var skippedGenerationsCount int
 	sourceHead, err := r.sourceRepo.HeadHash()
 	if err != nil {
 		return err
 	}
 	outputDir := filepath.Join(r.workRoot, "output")
 	timings := map[string]time.Duration{}
 	for _, libraryState := range r.state.Libraries {
+		if r.librarianConfig.IsGenerationBlocked(libraryState.ID) {
+			slog.Debug("skipping generation for library due to generate_blocked", "library", libraryState.ID)
+			skippedGenerationsCount++
+			continue
+		}
 		startTime := time.Now()
 		err := r.regenerateSingleLibrary(ctx, libraryState, outputDir)
 		if err != nil {
@@ -137,9 +143,14 @@ func (r *updateImageRunner) run(ctx context.Context) error {
 		timings[libraryState.ID] = time.Since(startTime)
 	}
 	if len(failedGenerations) > 0 {
-		slog.Warn("failed generations", slog.Int("num", len(failedGenerations)))
+		slog.Warn("failed generations", "num", len(failedGenerations))
 	}
-	slog.Info("successful generations", slog.Int("num", len(successfulGenerations)))
+	slog.Info(
+		"generation statistics",
+		"all", len(r.state.Libraries),
+		"successes", len(successfulGenerations),
+		"skipped", skippedGenerationsCount,
+		"failures", len(failedGenerations))
 	if err := writeTiming(r.workRoot, timings); err != nil {
 		return err
 	}

internal/librarian/update_image_test.go

@@ -194,6 +194,7 @@ func TestUpdateImageRunnerRun(t *testing.T) {
 		containerClient            *mockContainerClient
 		ghClient                   *mockGitHubClient
 		state                      *config.LibrarianState
+		librarianConfig            *config.LibrarianConfig
 		image                      string
 		build                      bool
 		commit                     bool
@@ -728,6 +729,36 @@ func TestUpdateImageRunnerRun(t *testing.T) {
 			wantCreateIssueCalls:       1,
 			wantCommitMsg:              "feat: update image to gcr.io/test/image@sha256:abc123",
 		},
+		{
+			name: "skip generation for library",
+			state: &config.LibrarianState{
+				Image: "gcr.io/test/image:v1.2.3",
+				Libraries: []*config.LibraryState{
+					{
+						ID:                  "blocked-lib",
+						APIs:                []*config.API{{Path: "some/api1"}},
+						SourceRoots:         []string{"src/a"},
+						LastGeneratedCommit: "abcd1234",
+					},
+				},
+			},
+
+			librarianConfig: &config.LibrarianConfig{
+				Libraries: []*config.LibraryConfig{
+					{
+						LibraryID:       "blocked-lib",
+						GenerateBlocked: true,
+					},
+				},
+			},
+			containerClient:     &mockContainerClient{},
+			imagesClient:        &mockImagesClient{},
+			ghClient:            &mockGitHubClient{},
+			wantFindLatestCalls: 1,
+			wantGenerateCalls:   0,
+			wantBuildCalls:      0,
+			wantCheckoutCalls:   1,
+		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			testRepo := newTestGitRepoWithState(t, test.state)
@@ -755,6 +786,7 @@ func TestUpdateImageRunnerRun(t *testing.T) {
 				imagesClient:    test.imagesClient,
 				ghClient:        test.ghClient,
 				state:           test.state,
+				librarianConfig: test.librarianConfig,
 				workRoot:        t.TempDir(),
 				repo:            repo,
 				sourceRepo:      sourceRepo,